Remove the SqliteReportBuilderTx indirection
#8
Conversation
Swatinem
changed the title
BuilderTx indirectionSqliteReportBuilderTx indirection
|
you're right that this is untidy, but there's a lifetime issue that makes doing what we need to do here tricky we actually want this was a little while ago so i don't remember my whole thought process, but iirc i thought moving ownership of the i don't like where i left this though, so changes are welcome |
matt-codecov
force-pushed
the
matt/replace-uuid-and-batch-insert
branch
from
c26f8f3 to
c434444
Compare
This removes the `SqliteReportBuilderTx` as it was effectively unused and an unnecessary indirection. All users were going through the main `SqliteReportBuilder`, which was using a transaction for each method anyway. Depending on the plans for error handling, a whole report (builder) should be constructed without errors for it to be internally consistent anyway, so the ability to rollback transactions with an arbitrary granularity does not make sense for now.
Swatinem
force-pushed
the
swatinem/rm-transaction
branch
from
00df4ae to
f8c6d63
Compare
|
Oh, I see that I missed this primary usage when looking at it. I will close this, as it does not make sense. |
This removes the
SqliteReportBuilderTxas it was effectively unused and an unnecessary indirection.All users were going through the main
SqliteReportBuilder, which was using a transaction for each method anyway.Depending on the plans for error handling, a whole report (builder) should be constructed without errors for it to be internally consistent anyway, so the ability to rollback transactions with an arbitrary granularity does not make sense for now.